-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New wordwrap #9649
New wordwrap #9649
Conversation
proc `$`*(rune: Rune): string = | ||
## Converts a Rune to a string | ||
rune.toUTF8 | ||
|
||
proc `$`*(runes: seq[Rune]): string = | ||
## Converts a sequence of Runes to a string | ||
result = "" | ||
for rune in runes: result.add(rune.toUTF8) | ||
for rune in runes: | ||
result.add rune |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
|
||
proc wordWrap*(s: string, maxLineWidth = 80, | ||
splitLongWords = true, | ||
newLine = "\n"): string = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two spaces before the '='
## whitespace is treated equally. Non-breaking whitespace is | ||
## ignored. | ||
|
||
var currentWordLength: int = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use type inference, ': int' is annoying.
## ignored. | ||
|
||
var currentWordLength: int = 0 | ||
var currentWord: string = newStringOfCap(32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise for these.
var currentWordLengthAtLineEnd: int = -1 | ||
var longWordMode = false | ||
|
||
template handleWhitespace(): untyped = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that should be better named addWord
.
currentLineLength = 0 | ||
|
||
if currentLineLength > 0: | ||
result.add ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, don't nuke the whitespace replacing it by a single space.
@@ -0,0 +1,67 @@ | |||
import unicode | |||
|
|||
proc wordWrap*(s: string, maxLineWidth = 80, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
procs use verbs, so the name should be wrapWords
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't make sense. "wordwrap" can be a verb too. Using your argument, we'd then have "echoes", "aligns", "adds", etc.
Sorry, I went bonkers because I thought you suggested "wordwraps" instead of "wrapwords".
I agree that "wrapwords" is better (not because of thr verb argument; wordwraps can be read as a verb too), but because of correctness.. we are wrapping many "words" across lines, not just a single word.
if currentWord.len > 0: | ||
|
||
if currentLineLength + 1 + currentWordLength > maxLineWidth: | ||
result.add newLine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result.add newLine | |
if len(result) > 0: | |
result.add newLine |
otherwise you are inserting an empty line, if the length of the first word on the first line is >= maxLineWidth.
This wordwrap.wordWrap("abc\ndef") == "abc def" while strutils.wordWrap("abc\ndef") == "abc\ndef" is this intentional? |
@skilchen for me this change is intentional, but it seems Araq disagrees on it. |
Did it my way based on your work. |
this is supposed to fix #3196. I have a question about the name though. The function does more than just wrapping words, it fills up the lines so that you don't end up with lines with just a single word in them when a newline is inserted. In emacs uses function names such as
fill-paragraph
andfill-region
.